Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add guest package for fetching attestation report via syscall #1341

Merged
merged 2 commits into from
Apr 13, 2022
Merged

Add guest package for fetching attestation report via syscall #1341

merged 2 commits into from
Apr 13, 2022

Conversation

anmaxvl
Copy link
Contributor

@anmaxvl anmaxvl commented Mar 31, 2022

Add ioctl wrapper and structs required to fetch attestation report.

Add a command line tool to fetch real and fake attestation
reports.
Fake attestation report can be used when testing integrations.

Signed-off-by: Maksim An maksiman@microsoft.com

@anmaxvl anmaxvl requested a review from a team as a code owner March 31, 2022 07:13
@anmaxvl
Copy link
Contributor Author

anmaxvl commented Mar 31, 2022

@SeanTAllen @KenGordon @svolos FYI.

@dcantah
Copy link
Contributor

dcantah commented Mar 31, 2022

This is the replacement for #1329?

@anmaxvl
Copy link
Contributor Author

anmaxvl commented Mar 31, 2022

This is the replacement for #1329?

yeah. #1329 was opened as a reference C implementation.

helsaawy
helsaawy approved these changes Mar 31, 2022
View reviewed changes
internal/guest/snp/report.go Outdated Show resolved Hide resolved
internal/tools/snp-report/main.go Outdated Show resolved Hide resolved
internal/guest/snp/fake_report.go Outdated Show resolved Hide resolved
internal/guest/snp/report.go Outdated Show resolved Hide resolved
@KenGordon
Copy link
Collaborator

Github doesn't want me to reply to the question about formats, but a json version isn't totally useful as the binary data is signed. Change the format and checking the signature becomes difficult. As a client I cannot rely on, for example, the hostdata or boot measurement fields unless I can validate that the whole report is real and produced by a real device.

@anmaxvl anmaxvl force-pushed the snp-report branch 2 times, most recently from 17e266b to d86bdfc Compare April 1, 2022 17:09
kevpar
kevpar reviewed Apr 1, 2022
View reviewed changes
internal/guest/snp/report.go Outdated Show resolved Hide resolved
kevpar
kevpar reviewed Apr 1, 2022
View reviewed changes
internal/guest/snp/report.go Outdated Show resolved Hide resolved
kevpar
kevpar reviewed Apr 1, 2022
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@KenGordon
Copy link
Collaborator

KenGordon commented Apr 3, 2022 via email

@anmaxvl anmaxvl force-pushed the snp-report branch 4 times, most recently from 378d00e to 47a2fcf Compare April 4, 2022 06:07
kevpar
kevpar reviewed Apr 4, 2022
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@kevpar
Copy link
Member

kevpar commented Apr 4, 2022

It looks like you're checking in the binary internal/tools/snp-report/snp-report. Probably not intended?

@anmaxvl
Copy link
Contributor Author

anmaxvl commented Apr 4, 2022

It looks like you're checking in the binary internal/tools/snp-report/snp-report. Probably not intended?

thanks for catching this! definitely not!

@anmaxvl anmaxvl force-pushed the snp-report branch 3 times, most recently from 2cddb4d to c74e7c2 Compare April 7, 2022 17:19
kevpar
kevpar reviewed Apr 8, 2022
View reviewed changes
Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of nits and one concern about the data encoding. Almost ready to go in I think!

@KenGordon
Copy link
Collaborator

KenGordon commented Apr 8, 2022 via email

@KenGordon
Copy link
Collaborator

KenGordon commented Apr 8, 2022 via email

@KenGordon
Copy link
Collaborator

KenGordon commented Apr 9, 2022 via email

kevpar
kevpar approved these changes Apr 12, 2022
View reviewed changes
Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kevpar
Copy link
Member

kevpar commented Apr 12, 2022

Make sure you either rebase your commits into a curated set, or use "Squash and merge".

kevpar
kevpar reviewed Apr 12, 2022
View reviewed changes
pkg/amdsev/report.go Outdated Show resolved Hide resolved
@anmaxvl
Add guest package for fetching attestation report via syscall
2b3092f 
Add internal/guest/linux package, which contains linux ioctl
definitions. Devicemapper code is refactored to use the new package.
Introduce ioctl wrapper and structs required to fetch attestation
report.
Validate that LaunchData provided to HCS and HostData returned as
part of attestation report match.

Add utility binary to fetch SNP report and update Makefile to
support DEV_BUILD parameter, which includes test utilities inside
LCOW image.
Fake attestation report can be used when testing integrations.

Signed-off-by: Maksim An <maksiman@microsoft.com>
@anmaxvl
make amdsevsnp a public package.
ddb7af2 
Signed-off-by: Maksim An <maksiman@microsoft.com>
kevpar
kevpar approved these changes Apr 13, 2022
View reviewed changes
Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@msscotb
Copy link
Contributor

msscotb commented Apr 13, 2022

lgtm

@anmaxvl anmaxvl merged commit 13ceffd into microsoft:master Apr 13, 2022
@anmaxvl anmaxvl deleted the snp-report branch April 13, 2022 19:51
anmaxvl added a commit that referenced this pull request Feb 7, 2023
@anmaxvl
Merged PR 5999316: Update ADO to dbb347e
437abac 
Sync ADO with upstream to enable including test GCS binaries as
part of dev-pipeline

Related work items: #1311, #1322, #1341, #1343, #1345, #1347, #1348, #1350, #1353, #1354, #1355, #1358, #1361, #1365, #1368, #1369, #1370
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
@anmaxvl
Add guest package for fetching attestation report via syscall (micros…
7d44757 
…oft#1341)

Add `internal/guest/linux package`, which contains linux ioctl
definitions. Devicemapper code is refactored to use the new package.
Introduce new `amdsevsnp` package with Introduce ioctl wrappers and
structs required to fetch attestation report.
Validate that `LaunchData` provided to HCS during UVM boot and
`HostData` returned as part of attestation report match.

Add utility binary to fetch SNP report and update Makefile to
support `DEV_BUILD` parameter, which includes test utilities inside
LCOW image.
Fake attestation report can be used when testing integrations.

Signed-off-by: Maksim An <maksiman@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcantah dcantah dcantah left review comments

@svolos svolos svolos left review comments

@helsaawy helsaawy helsaawy approved these changes

@msscotb msscotb msscotb approved these changes

@kevpar kevpar kevpar approved these changes

Assignees

@kevpar kevpar

@dcantah dcantah

@msscotb msscotb

@helsaawy helsaawy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@anmaxvl @dcantah @KenGordon @kevpar @msscotb @svolos @helsaawy